make vlan check conditional on management network for xen #11193
Conversation
|
@blueorangutan package |
|
@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #11193 +/- ##
============================================
+ Coverage 16.15% 16.58% +0.42%
- Complexity 13273 13992 +719
============================================
Files 5656 5745 +89
Lines 497728 510757 +13029
Branches 60360 62144 +1784
============================================
+ Hits 80420 84688 +4268
- Misses 408357 416599 +8242
- Partials 8951 9470 +519
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 14173 |
|
@blueorangutan test ol9 xcpng83 |
|
@DaanHoogland a [SL] Trillian-Jenkins test job (ol9 mgmt + xcpng83) has been kicked to run smoke tests |
|
[SF] Trillian test result (tid-13775)
|
|
@UAnton can you test if this satisfies your use-case, please? |
I can't. Too much time has passed |
ok, should we close the issue than? |
There was a problem hiding this comment.
Pull Request Overview
This PR modifies the VLAN validation logic for XenServer management networks to only apply the check for XenServer API versions prior to version 8. The change makes the VLAN configuration check conditional based on the XenServer API version, allowing management networks on VLANs for newer XenServer versions while maintaining the restriction for older versions.
- Adds API version check to conditionally enforce VLAN restrictions on management networks
| @@ -104,14 +104,14 @@ public Answer execute(final SetupCommand command, final CitrixResourceBase citri | |||
| for (final PIF pif : hostPifs) { | |||
| final PIF.Record rec = pif.getRecord(conn); | |||
| if (rec.management) { | |||
| if (rec.VLAN != null && rec.VLAN != -1) { | |||
| if (host.getAPIVersionMajor(conn) < 8 && rec.VLAN != null && rec.VLAN != -1) { | |||
There was a problem hiding this comment.
The API version check should include proper null safety. Consider checking if host.getAPIVersionMajor(conn) returns a valid value before comparison to prevent potential NullPointerException.
| if (host.getAPIVersionMajor(conn) < 8 && rec.VLAN != null && rec.VLAN != -1) { | |
| Integer apiVersionMajor = host.getAPIVersionMajor(conn); | |
| if (apiVersionMajor == null) { | |
| final String msg = "Unable to determine API version for host: " + citrixResourceBase.getHost().getUuid(); | |
| logger.warn(msg); | |
| return new SetupAnswer(command, msg); | |
| } | |
| if (apiVersionMajor < 8 && rec.VLAN != null && rec.VLAN != -1) { |
Copilot uses AI. Check for mistakes.
|
@sureshanaparti , if there is no ask for this, I am not sure if we should put effort in it. I am not full of confidence on this implementation. |
Description
This PR...
Fixes: #10272
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?